Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Base64 encode serialized jobs when using Postgres #51

Merged
merged 6 commits into from
Apr 16, 2022

Conversation

ksassnowski
Copy link
Owner

Fixes #42

This PR fixes a bug where serializing workflow jobs with private properties could lead to truncated data being saved when using Postgresql.

Since we serialize and unserialize workflow jobs in multiple places (the Workflow model, as well as the UnserializeWorkflowJobExtractor), I decided to separate this logic out into a separate class so it isn't duplicated all over the place.

In short, when serializing workflow job, we check if we’re using a PostgresConnection and if so base64_encode the serialized string before storing it. This gets rid of any null bytes in the serialized string so Postgres doesn’t treat it as a null-terminated string anymore. We do the inverse when unserializing jobs. To ensure backwards compatibility, we don't base64_decode a string if it doesn't contain : or ;. We use this as a heuristic to check if we're dealing with a base64 encoded string or not since these characters will never appear in a base64 encoded string. This implementation is based on the PR which fixes the same bug in Laravel’s queue system (laravel/framework#36081).

I haven’t added a separate test to test for this specific bug, as adding configuring a Postgres database in our CI pipeline just for a single test seemed a little excessive. I did, however, reproduce the bug locally and confirmed that these changes fix the bug. I used the example repository provided in #42 as starting point.

As far as I can tell, this change should be backwards compatible and should not require a major version bump.

@ksassnowski ksassnowski merged commit c0b28e2 into master Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Postgresql] Serialization/Unserialization error while using non-public fields on jobs
2 participants